Skip to content

feat(logging): add spdlog backend behind ICEBERG_SPDLOG (5/6)#726

Open
kamcheungting-db wants to merge 6 commits into
apache:mainfrom
kamcheungting-db:logging-block5-spdlog
Open

feat(logging): add spdlog backend behind ICEBERG_SPDLOG (5/6)#726
kamcheungting-db wants to merge 6 commits into
apache:mainfrom
kamcheungting-db:logging-block5-spdlog

Conversation

@kamcheungting-db

@kamcheungting-db kamcheungting-db commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Part 5 of the logging stack (builds on #725). Adds the spdlog backend and the build option that selects it — the default in production builds.

What's here

  • SpdLogger wraps spdlog::logger (synchronous), maps levels (fatal/critical → spdlog critical; the abort stays in the macro layer), and forwards source location. Default sink is a colored stderr sink.
  • New ICEBERG_SPDLOG CMake option (ON by default). When OFF, the build has no spdlog dependency at all and CerrLogger is the default.
  • pattern property is honored here via spdlog set_pattern (the cerr backend keeps its fixed layout); level works on both backends.
  • SpdLogger lives in logging/internal/ and is not installed — apps get it via the default logger or the "spdlog" registry type.

Testsspdlog_logger_test (compiled only when spdlog is ON): level mapping incl. fatal→critical, source-location forwarding, and the pattern property. clang/libc++ with spdlog 1.15.3.

This pull request and its description were written by Isaac.

@kamcheungting-db kamcheungting-db changed the title feat: [Iceberg Logger] [Part-5] spdlog backend (ICEBERG_SPDLOG) feat(logging): add spdlog backend behind ICEBERG_SPDLOG Jun 11, 2026
@kamcheungting-db kamcheungting-db changed the title feat(logging): add spdlog backend behind ICEBERG_SPDLOG feat(logging): add spdlog backend behind ICEBERG_SPDLOG (5/6) Jun 11, 2026
@kamcheungting-db
kamcheungting-db force-pushed the logging-block5-spdlog branch 2 times, most recently from c6831cb to 2b80ee1 Compare June 14, 2026 06:16
@kamcheungting-db
kamcheungting-db force-pushed the logging-block5-spdlog branch 14 times, most recently from dd50f86 to 17a616e Compare June 22, 2026 10:24
@kamcheungting-db
kamcheungting-db force-pushed the logging-block5-spdlog branch 6 times, most recently from 7e81c6d to 2684d7c Compare June 24, 2026 18:39
@kamcheungting-db
kamcheungting-db force-pushed the logging-block5-spdlog branch 2 times, most recently from 289b0fb to ef127cd Compare June 30, 2026 20:36
Fourth block: the application-facing macros, the only part most callers touch.

- ICEBERG_LOG_{TRACE,DEBUG,INFO,WARN,ERROR,CRITICAL,FATAL} plus the generic
  ICEBERG_LOG(level, ...), ICEBERG_LOG_TO(logger, level, ...) for an explicit
  logger, and ICEBERG_LOG_RUNTIME_FMT for a runtime (non-literal) format string.
- ICEBERG_LOG_ACTIVE_LEVEL is a compile-time severity floor: statements below it
  are removed entirely via `if constexpr` (no format call site, no source
  location emitted). ICEBERG_LOG_FATAL is never gated by the floor -- its abort
  is always compiled in; it emits, best-effort Flush()es the same logger it
  emitted to, then std::abort().
- Filtering is decided solely by Logger::ShouldLog(); formatting is wrapped in
  try/catch so logging never throws (a format failure routes to EmitFormatError).
- Bare Java-style aliases (LOG_INFO, ...) are opt-in via ICEBERG_LOG_SHORT_MACROS
  to avoid polluting consumers / colliding with glog/abseil.

Header-only addition to logger.h. macros_test covers injection, the
guard-before-format short-circuit, never-throws, and FATAL aborts;
macros_active_level_test verifies compile-time stripping in a kOff translation unit.

Co-authored-by: Isaac
Note in FormatAndEmit and the ICEBERG_INTERNAL_LOG comment that the only throws
are std::format_error/std::bad_alloc (same recovery), and that catching
std::exception rather than (...) lets a non-std unwind such as
abi::__forced_unwind (thread cancellation) propagate.

Co-authored-by: Isaac
… docs, FATAL test)

- Revert the 6 catch sites to catch(...) so the noexcept "logging never throws"
  guarantee holds even when a user-defined std::formatter throws a non-std type
  (catch(const std::exception&) would let it reach std::terminate).
- Apply /Zc:preprocessor to the iceberg library targets PUBLICly on MSVC, not
  only to tests: logger.h is public and uses __VA_OPT__, so the library build and
  consumers need the conforming preprocessor too.
- Soften the compile-time-floor doc: `if constexpr` discards the emit but the
  statement must still be well-formed (bad format string is still a compile error).
- Add the missing opening '[' to the logger.h example output lines to match
  CerrLogger's [file:line] layout.
- Add MacrosDeathTest.FatalRoutesThroughScopedLogger locking in that
  ICEBERG_LOG_FATAL routes through the active ScopedLogger (GetCurrentLogger).

Co-authored-by: Isaac
Adopt the structure from the apache#824 prototype (keeping our if constexpr gating):
- logger.h is now the C++ API only; ICEBERG_LOG_* macros move to log_macros.h,
  and the bare LOG_* aliases move behind an opt-in short_log_macros.h (replacing
  the ICEBERG_LOG_SHORT_MACROS define).
- Dedup the five macro bodies onto shared internal helpers (EmitIfEnabled +
  LogToCurrent / LogToCurrentRuntime / LogToExplicitRuntime / LogFatal) that take
  a lazy [&]()->std::string message thunk invoked only past ShouldLog, so disabled
  logs still don't evaluate their args. Keeps catch(...), the GetCurrentLogger()
  fatal path, and if constexpr compile-time floor.
- VFormat moves to log_macros.h (only ICEBERG_LOG_RUNTIME_FMT uses it).

Co-authored-by: Isaac
@kamcheungting-db
kamcheungting-db force-pushed the logging-block5-spdlog branch 2 times, most recently from 864daa0 to db413fa Compare July 16, 2026 07:09
…review ⑤)

Address the extensibility comment: let an embedder (JNI, Python host, crash
reporter) run a hook on the fatal path before std::abort() to flush resources or
print a stack trace.

- Add FatalHandler = std::function<void(const std::source_location&,
  std::string_view)> plus thread-safe SetFatalHandler/GetFatalHandler (leaked,
  teardown-safe slot in logger.cc).
- Wire it into LogFatal: format the message once, emit-if-enabled + flush, run the
  handler (message passed even when the record is filtered out), then abort. A
  throwing handler cannot prevent the abort.
- Death tests: handler runs with the formatted message, receives the call-site
  location, and still fires when the record is suppressed.

Co-authored-by: Isaac
Fifth block: the default production backend and the build option that selects it.

- SpdLogger wraps spdlog::logger (kCritical/kFatal -> spdlog critical, others 1:1),
  forwarding the pre-formatted message and source location. Synchronous only in
  v1 (spdlog's source_loc is a non-owning const char*, unsafe with async sinks).
  It lives in logging/internal/, is gated by #ifdef ICEBERG_HAS_SPDLOG, and is
  NOT installed -- consumers obtain it via the default logger or the registry,
  never by including spdlog headers.
- New ICEBERG_SPDLOG CMake option (default ON). config.h is ALWAYS generated
  (only ICEBERG_HAS_SPDLOG's definedness varies) so logger.cc compiles in both
  configurations; MakeDefaultLogger() prefers SpdLogger when compiled in, else
  CerrLogger.
- Critically, ICEBERG_SPDLOG=OFF now UNWIRES the previously-unconditional spdlog
  link (interface-lib lists + resolve_spdlog_dependency), not just the new
  source -- so an OFF build has no spdlog dependency at all.

spdlog_logger_test (compiled only on the ON path) covers the level mapping
including fatal->critical and source-location forwarding.

Co-authored-by: Isaac

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds a selectable spdlog-backed logging sink to Iceberg C++ (guarded by ICEBERG_SPDLOG in CMake), introduces the public logging macro headers (log_macros.h / short_log_macros.h), and extends the test/build wiring to cover macro behavior and the spdlog backend.

Changes:

  • Add internal::SpdLogger (spdlog backend) and route the process default logger to it when ICEBERG_HAS_SPDLOG is enabled.
  • Introduce installed logging macro headers (ICEBERG_LOG_*, plus opt-in bare LOG_* aliases) and MSVC preprocessor flags needed for __VA_OPT__.
  • Add unit tests for logging macros and the spdlog backend; update CMake/Meson build definitions accordingly.

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
src/iceberg/test/spdlog_logger_test.cc Adds unit tests for the spdlog-backed logger behavior.
src/iceberg/test/meson.build Includes the new logging-related tests in the Meson test target.
src/iceberg/test/macros_test.cc Adds tests for formatting, level gating, runtime-format safety, and fatal-abort behavior in macros.
src/iceberg/test/macros_active_level_test.cc Adds tests for compile-time log stripping via ICEBERG_LOG_ACTIVE_LEVEL.
src/iceberg/test/CMakeLists.txt Wires the new tests into CMake and adds MSVC /Zc:preprocessor for test compilation.
src/iceberg/meson.build Compiles the spdlog backend source in the Meson build.
src/iceberg/logging/short_log_macros.h Adds opt-in bare LOG_* aliases for the Iceberg-prefixed macros.
src/iceberg/logging/meson.build Generates logging/config.h and installs the new public logging headers in Meson.
src/iceberg/logging/logger.h Extends the logging API docs and adds the FatalHandler hook API.
src/iceberg/logging/logger.cc Selects spdlog vs CerrLogger as the default logger based on ICEBERG_HAS_SPDLOG and implements fatal-handler storage.
src/iceberg/logging/log_macros.h Introduces the public logging macros and internal helpers used by those macros.
src/iceberg/logging/internal/spdlog_logger.h Declares the internal spdlog-backed SpdLogger sink (not installed).
src/iceberg/logging/internal/spdlog_logger.cc Implements the spdlog-backed sink (pattern support, level mapping, forwarding).
src/iceberg/logging/config.h.in Adds the build-generated backend selection header template (ICEBERG_HAS_SPDLOG).
src/iceberg/CMakeLists.txt Generates iceberg/logging/config.h, gates spdlog linkage/compilation behind ICEBERG_SPDLOG, and exports /Zc:preprocessor on MSVC.
meson.build Adds /Zc:preprocessor to MSVC project arguments for Meson builds.
CMakeLists.txt Introduces the ICEBERG_SPDLOG option (default ON).
cmake_modules/IcebergThirdpartyToolchain.cmake Avoids resolving the spdlog dependency when ICEBERG_SPDLOG is OFF.

Comment on lines +99 to +108
template <typename MakeMessage>
void LogToExplicitRuntime(Logger& logger, LogLevel level,
const std::source_location& location,
MakeMessage&& make_message) noexcept {
EmitIfEnabled(logger, level, location, std::forward<MakeMessage>(make_message));
if (level == LogLevel::kFatal) {
logger.Flush();
std::abort();
}
}
Comment on lines +65 to +72
Status SpdLogger::Initialize(
const std::unordered_map<std::string, std::string>& properties) {
if (auto it = properties.find(std::string(kPatternProperty)); it != properties.end()) {
logger_->set_pattern(it->second);
}
// Apply "level" via the base implementation.
return Logger::Initialize(properties);
}
Comment on lines +81 to +92
void SpdLogger::Log(LogMessage&& message) noexcept {
try {
spdlog::source_loc loc{message.location.file_name(),
static_cast<int>(message.location.line()),
message.location.function_name()};
// Pass the pre-formatted text as an argument ("{}") so any braces in the
// message are not re-interpreted as a format string.
logger_->log(loc, ToSpdLevel(message.level), "{}", message.message);
} catch (...) {
// Logging must never throw.
}
}
Comment on lines +94 to +99
void SpdLogger::Flush() noexcept {
try {
logger_->flush();
} catch (...) {
}
}
Comment on lines +25 to +31
/// INTERNAL, NOT INSTALLED. It is only included from .cc files (logger.cc and
/// spdlog_logger.cc) after config.h, and only when the project is built with
/// ICEBERG_SPDLOG=ON. SpdLogger is not a consumer-constructible public type --
/// applications obtain it via the default logger or the "logger-impl"="spdlog"
/// registry factory.

#include "iceberg/logging/config.h"
Comment on lines +21 to +30
# Generate the logging backend config header. ALWAYS generated (not gated by
# ICEBERG_SPDLOG) so logging/logger.cc can include it in both ON and OFF builds;
# only the definedness of ICEBERG_HAS_SPDLOG varies. Generated into the build
# tree (already on ICEBERG_INCLUDES), included as "iceberg/logging/config.h", and
# NOT installed (it must never appear in a public/installed header).
if(ICEBERG_SPDLOG)
set(ICEBERG_HAS_SPDLOG ON)
endif()
configure_file("${CMAKE_CURRENT_SOURCE_DIR}/logging/config.h.in"
"${CMAKE_CURRENT_BINARY_DIR}/logging/config.h")
Comment on lines +84 to +95
template <typename MakeMessage>
void LogToCurrentRuntime(LogLevel level, const std::source_location& location,
MakeMessage&& make_message) noexcept {
const std::shared_ptr<Logger>& logger = CurrentLogger();
if (logger) {
EmitIfEnabled(*logger, level, location, std::forward<MakeMessage>(make_message));
}
if (level == LogLevel::kFatal) {
if (logger) logger->Flush();
std::abort();
}
}
@manuzhang manuzhang added this to the 0.4.0 milestone Jul 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants